Skip to content

Conversation

@soyuka
Copy link
Contributor

@soyuka soyuka commented Oct 18, 2025

Refactor capability registration in Server Builder using dedicated loaders. A solution for #74

Motivation and Context

The previous Server/Builder.php contained extensive logic for registering capabilities (tools, resources, prompts) both manually and through discovery. This led to a large and less modular Builder class. This change aims to improve the architecture by introducing dedicated loader classes for capability registration.

How Has This Been Tested?

Existing tests for server capabilities and discovery should cover the functionality. No new tests were added as this is a refactoring. Let me know if I should add some as I'm not sure this is going to be accepted I didn't wanted to put unnecessary energy into unit testing.

Breaking Changes

No breaking changes are expected as this is an internal refactoring. The public API of the Server/Builder remains the same.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

This refactoring extracts the capability registration logic into ArrayRegistryLoader for manual registrations and DiscoveryRegistryLoader for discovery-based registrations. Both implement RegistryLoaderInterface. The Server/Builder now orchestrates these loaders, leading to a more extensible design.

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @soyuka - happy to see you here! :)

I think this PR makes a lot of sense and sorts some issues we saw already, thanks for that! Only minor comments from my end - didn't give it a test tho.

@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Oct 20, 2025
@chr-hertel chr-hertel changed the title refactor: registry loader [Server] refactor: registry loader Oct 20, 2025
@soyuka
Copy link
Contributor Author

soyuka commented Oct 24, 2025

my bad forgot to self-review I've also removed the empty calls and set sane default values in the constructor.

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one - thanks @soyuka!

@chr-hertel chr-hertel merged commit e4a82f7 into modelcontextprotocol:main Oct 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants